fixes issue 17162 false positive for never type impls#17163
fixes issue 17162 false positive for never type impls#17163durationextender wants to merge 1 commit into
Conversation
rustbot
added
the
S-waiting-on-review
|
r? @Jarcho rustbot has assigned @Jarcho. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@rustbot ready |
|
This should at least make sure that the trait method in question actually has will successfully compile and print "hi!!". This would be nice to add as a test case. But before you do: I'm not actually sure this is the correct fix for the issue at hand, let me discuss that under the issue. |
| && let Some(builtin_crate) = clippy_utils::std_or_core(cx) | ||
| && let Some(inner) = unpack_async_fn_body(cx, body) | ||
| // Find the tail expression contained in the async fn (if any), | ||
| // which will be wrapped in std::future::ready. | ||
| && let ExprKind::Block(block, _) = inner.kind | ||
| && let Some(tail_expr) = block.expr |
There was a problem hiding this comment.
These shouldn't be changed.
There was a problem hiding this comment.
idk why those got changed tbh maybe because of cargo dev fmt?
| let parent = cx.tcx.hir_get_parent_item(impl_item.hir_id()).def_id; | ||
| let item = cx.tcx.hir_expect_item(parent); | ||
| let self_ty = cx.tcx.type_of(item.owner_id).instantiate_identity().skip_norm_wip(); | ||
| //check if self.ty is never type ! if it is, then we don't want to lint because the function can | ||
| // never be called and thus doesn't need to be async | ||
| if self_ty.is_never() { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The actual important thing to check is if any of the input types are uninhabited. You can use Ty::is_inhabited_from.
There was a problem hiding this comment.
That would mean that this check should be applied to freestanding functions as well, right? So it would probably make sense to extract it to a separate function, and call that both here and in check_fn
There was a problem hiding this comment.
okay i will change the self_ty.is_never() to use is_inhabited_from instead.
There was a problem hiding this comment.
could you perhaps explain that func? the is_inhabited_from? because there is only 1 reference of it in the whole codebase and that ref is very vague of what is expects imo
There was a problem hiding this comment.
Everything you need is in LateContext. You can get the DefId of the innermost enclosing item from cx.enclosing_body.unwrap().hir_id.owner.def_id.to_def_id() which can be used directly.
The function returns how the item passed views a type's inhabitedness. A struct containing a private uninhabited field can only be seen as uninhabited from items that can access that field.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
☔ The latest upstream changes (possibly #17181) made this pull request unmergeable. Please resolve the merge conflicts. |
4 participants
Do not lint
unused_async_trait_implforimpl Trait for !blocks.The never type
!can never be instantiated, so any method implementedfor it can never be called. Flagging these as having an "unused async"
is a false positive since the async keyword has no practical impact.
fixes #17162
changelog: [
unused_async_trait_impl]: do not lint when the trait is implemented for the never type!I ran the tests locally and pass, also did uibless and formatted the code